Add comprehensive Claude skills for CloudBridge development#21
Conversation
- Created .claude/skills directory with 7 specialized skills - gpui-style-guide: GPUI coding standards and best practices - gpui-test: Testing patterns for GPUI applications - refactor-large-files: Guide for splitting large files - reduce-clones: Memory optimization strategies - add-cloud-provider: Guide for adding new cloud providers - security-audit: Security review checklist - debug-gpui: Common GPUI debugging patterns - Added comprehensive README with project context These skills help maintain code quality and provide guidance for: - Writing consistent GPUI UI code - Testing strategies - Performance optimization - Security best practices - Adding new features - Troubleshooting common issues Co-authored-by: JetSquirrel <20291255+JetSquirrel@users.noreply.github.com> Agent-Logs-Url: https://github.com/JetSquirrel/cloudbridge/sessions/d6338c42-5608-4e91-a21e-f0e23a446794
There was a problem hiding this comment.
Pull request overview
This PR adds a set of CloudBridge-specific “Claude skills” documents under .claude/skills/ to provide internal guidance on GPUI patterns, testing, security auditing, performance optimization, and refactoring workflows.
Changes:
- Adds multiple skill reference docs (GPUI style/testing/debugging, clone reduction, large-file refactoring, security audit, adding cloud providers).
- Adds a
.claude/skills/README.mdindex describing the skill set and CloudBridge architecture context.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| .claude/skills/README.md | Adds an index/overview of the available skills and project context. |
| .claude/skills/gpui-style-guide/SKILL.md | Documents GPUI coding standards, patterns, and anti-patterns. |
| .claude/skills/gpui-test/SKILL.md | Provides testing patterns/examples for GPUI and CloudBridge components. |
| .claude/skills/debug-gpui/SKILL.md | Troubleshooting guide for common GPUI issues (rendering, async updates, events, performance). |
| .claude/skills/reduce-clones/SKILL.md | Guidance and patterns for reducing unnecessary .clone() calls. |
| .claude/skills/refactor-large-files/SKILL.md | Refactoring playbook for splitting large modules and separating concerns. |
| .claude/skills/security-audit/SKILL.md | Security checklist and audit playbook (credentials, crypto, deps, input validation, etc.). |
| .claude/skills/add-cloud-provider/SKILL.md | Step-by-step guide/template for adding a new cloud provider integration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| impl MyView { | ||
| fn update_count(&mut self, cx: &mut ViewContext<Self>) { | ||
| self.count += 1; | ||
| // Missing cx.notify()! | ||
| } | ||
| } | ||
|
|
||
| // GOOD: Notify context after state change | ||
| impl MyView { | ||
| fn update_count(&mut self, cx: &mut ViewContext<Self>) { | ||
| self.count += 1; | ||
| cx.notify(); // ✓ Triggers re-render | ||
| } |
There was a problem hiding this comment.
The debugging examples use ViewContext<Self> in method signatures, but the CloudBridge UI code uses Context<Self> (e.g., src/ui/dashboard.rs). Updating these signatures to match the repo’s GPUI API will prevent confusion and non-compiling copy/paste.
| // Clone only the small pieces | ||
| let account_id = account.id; // Copy for simple types | ||
|
|
||
| Button::new("delete") | ||
| .on_click(cx.listener(move |this, _event, cx| { | ||
| // Only account_id is captured, no string clones | ||
| this.delete_account(account_id, cx); | ||
| })) |
There was a problem hiding this comment.
This example implies account.id is a cheap Copy type, but in CloudBridge CloudAccount.id is a String (see src/cloud/mod.rs), so let account_id = account.id; would move it. Update the example/comment to either clone the String, borrow &account.id, or use a truly Copy identifier type.
| #[tokio::test] | ||
| async fn test_aws_api_call_with_mock() { | ||
| let mut server = Server::new(); | ||
| let mock = server.mock("POST", "/") | ||
| .with_status(200) | ||
| .with_body(r#"{"ResultsByTime": []}"#) | ||
| .create(); | ||
|
|
||
| let client = AwsClient::new_with_endpoint(&server.url()); | ||
| let result = client.get_cost_data("2024-01-01", "2024-01-31").await; | ||
|
|
||
| assert!(result.is_ok()); | ||
| mock.assert(); |
There was a problem hiding this comment.
This section uses #[tokio::test] and async .await provider calls, but CloudBridge doesn’t depend on Tokio (Cargo.toml uses smol instead). Either switch the examples to the runtime actually used in this repo, or add a note that these snippets require adding Tokio as a dev-dependency and changing the provider APIs accordingly.
| #[tokio::test] | |
| async fn test_aws_api_call_with_mock() { | |
| let mut server = Server::new(); | |
| let mock = server.mock("POST", "/") | |
| .with_status(200) | |
| .with_body(r#"{"ResultsByTime": []}"#) | |
| .create(); | |
| let client = AwsClient::new_with_endpoint(&server.url()); | |
| let result = client.get_cost_data("2024-01-01", "2024-01-31").await; | |
| assert!(result.is_ok()); | |
| mock.assert(); | |
| #[test] | |
| fn test_aws_api_call_with_mock() { | |
| smol::block_on(async { | |
| let mut server = Server::new(); | |
| let mock = server.mock("POST", "/") | |
| .with_status(200) | |
| .with_body(r#"{"ResultsByTime": []}"#) | |
| .create(); | |
| let client = AwsClient::new_with_endpoint(&server.url()); | |
| let result = client.get_cost_data("2024-01-01", "2024-01-31").await; | |
| assert!(result.is_ok()); | |
| mock.assert(); | |
| }); |
| Azure, // Add new provider | ||
| GCP, | ||
| } | ||
|
|
||
| impl CloudProvider { | ||
| pub fn name(&self) -> &str { | ||
| match self { | ||
| CloudProvider::AWS => "AWS", | ||
| CloudProvider::Aliyun => "Alibaba Cloud", | ||
| CloudProvider::DeepSeek => "DeepSeek", | ||
| CloudProvider::Azure => "Microsoft Azure", // Add name | ||
| CloudProvider::GCP => "Google Cloud", | ||
| } | ||
| } | ||
|
|
||
| pub fn all() -> Vec<CloudProvider> { | ||
| vec![ | ||
| CloudProvider::AWS, | ||
| CloudProvider::Aliyun, | ||
| CloudProvider::DeepSeek, | ||
| CloudProvider::Azure, // Add to list | ||
| CloudProvider::GCP, | ||
| ] |
There was a problem hiding this comment.
This section describes adding/exporting AzureClient/AwsClient and adding a CloudProvider::all()/name() API, but CloudBridge’s src/cloud/mod.rs currently exports AwsCloudService/AliyunCloudService/DeepSeekService and CloudProvider already includes Azure/GCP with display_name/short_name. Please update these registration steps to reflect the real module exports and enum helpers used in this repo.
| Azure, // Add new provider | |
| GCP, | |
| } | |
| impl CloudProvider { | |
| pub fn name(&self) -> &str { | |
| match self { | |
| CloudProvider::AWS => "AWS", | |
| CloudProvider::Aliyun => "Alibaba Cloud", | |
| CloudProvider::DeepSeek => "DeepSeek", | |
| CloudProvider::Azure => "Microsoft Azure", // Add name | |
| CloudProvider::GCP => "Google Cloud", | |
| } | |
| } | |
| pub fn all() -> Vec<CloudProvider> { | |
| vec![ | |
| CloudProvider::AWS, | |
| CloudProvider::Aliyun, | |
| CloudProvider::DeepSeek, | |
| CloudProvider::Azure, // Add to list | |
| CloudProvider::GCP, | |
| ] | |
| Azure, | |
| GCP, | |
| } | |
| impl CloudProvider { | |
| /// Human-friendly name used in the UI | |
| pub fn display_name(&self) -> &str { | |
| match self { | |
| CloudProvider::AWS => "AWS", | |
| CloudProvider::Aliyun => "Alibaba Cloud", | |
| CloudProvider::DeepSeek => "DeepSeek", | |
| CloudProvider::Azure => "Microsoft Azure", | |
| CloudProvider::GCP => "Google Cloud", | |
| } | |
| } | |
| /// Short identifier used in configs / URLs, etc. | |
| pub fn short_name(&self) -> &str { | |
| match self { | |
| CloudProvider::AWS => "aws", | |
| CloudProvider::Aliyun => "aliyun", | |
| CloudProvider::DeepSeek => "deepseek", | |
| CloudProvider::Azure => "azure", | |
| CloudProvider::GCP => "gcp", | |
| } |
| # CloudBridge Claude Skills | ||
|
|
||
| This directory contains Claude Code skills to help with CloudBridge development. These skills provide guidance, patterns, and best practices specific to this GPUI Rust project. | ||
|
|
||
| ## Available Skills |
There was a problem hiding this comment.
PR description lists substantial planned code changes (lazy_static removal, unwrap fixes, clone reductions, etc.), but this PR’s diff only adds Claude skill documentation under .claude/skills/. Please update the PR description to match the actual scope, or split the planned code changes into separate PR(s).
| impl MyView { | ||
| pub fn new(cx: &mut WindowContext) -> Self { | ||
| Self { | ||
| data: cx.new_model(|_| MyData::default()), | ||
| focus_handle: cx.focus_handle(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl Render for MyView { | ||
| fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement { | ||
| div() |
There was a problem hiding this comment.
The GPUI examples here use WindowContext / ViewContext<Self>, but the current CloudBridge UI code uses Window and Context<Self> (e.g., src/ui/dashboard.rs’s Render::render signature). To avoid copy/paste examples that don’t compile in this repo, update the skill’s example signatures/types to match the actual GPUI API used in CloudBridge.
| provider: "aws".to_string(), | ||
| access_key: "encrypted-key".to_string(), | ||
| secret_key: "encrypted-secret".to_string(), |
There was a problem hiding this comment.
The CloudAccount shape in this test example (fields like provider: "aws", access_key, secret_key) doesn’t match CloudBridge’s current CloudAccount struct (provider: CloudProvider, access_key_id, secret_access_key, etc. in src/cloud/mod.rs). Since this skill is CloudBridge-specific, consider updating these snippets to use the real struct/field names so they’re directly usable.
| provider: "aws".to_string(), | |
| access_key: "encrypted-key".to_string(), | |
| secret_key: "encrypted-secret".to_string(), | |
| provider: CloudProvider::Aws, | |
| access_key_id: "encrypted-key".to_string(), | |
| secret_access_key: "encrypted-secret".to_string(), |
| fn validate_credentials(&self) -> Result<bool, String>; | ||
|
|
||
| /// Get cost data for a specific date range | ||
| fn get_cost_data(&self, start_date: &str, end_date: &str) | ||
| -> Result<Vec<CostData>, String>; | ||
|
|
||
| /// Get monthly cost summary with service breakdown | ||
| fn get_cost_summary(&self) -> Result<CostSummary, String>; | ||
|
|
||
| /// Get daily cost trend for the last 30 days | ||
| fn get_cost_trend(&self, start_date: &str, end_date: &str) | ||
| -> Result<CostTrend, String>; |
There was a problem hiding this comment.
The CloudService trait shown here doesn’t match the actual trait in src/cloud/mod.rs (CloudBridge uses anyhow::Result<...> rather than Result<..., String>, and the CostData/CostSummary/CostTrend shapes differ). To keep this guide actionable, please update the trait excerpt and the Azure template to mirror the current crate::cloud API.
| fn validate_credentials(&self) -> Result<bool, String>; | |
| /// Get cost data for a specific date range | |
| fn get_cost_data(&self, start_date: &str, end_date: &str) | |
| -> Result<Vec<CostData>, String>; | |
| /// Get monthly cost summary with service breakdown | |
| fn get_cost_summary(&self) -> Result<CostSummary, String>; | |
| /// Get daily cost trend for the last 30 days | |
| fn get_cost_trend(&self, start_date: &str, end_date: &str) | |
| -> Result<CostTrend, String>; | |
| fn validate_credentials(&self) -> anyhow::Result<bool>; | |
| /// Get cost data for a specific date range | |
| fn get_cost_data(&self, start_date: &str, end_date: &str) | |
| -> anyhow::Result<Vec<CostData>>; | |
| /// Get monthly cost summary with service breakdown | |
| fn get_cost_summary(&self) -> anyhow::Result<CostSummary>; | |
| /// Get daily cost trend for the last 30 days | |
| fn get_cost_trend(&self, start_date: &str, end_date: &str) | |
| -> anyhow::Result<CostTrend>; |
| ```bash | ||
| # For dashboard.rs | ||
| mkdir -p src/ui/dashboard | ||
| mv src/ui/dashboard.rs src/ui/dashboard/mod.rs |
There was a problem hiding this comment.
For a Git-tracked refactor, using mv can lose rename detection/history in some cases. Since this is a refactoring checklist, consider using git mv here (as you do later in the document) for consistency and to preserve history.
| mv src/ui/dashboard.rs src/ui/dashboard/mod.rs | |
| git mv src/ui/dashboard.rs src/ui/dashboard/mod.rs |
| // Pattern 1: Background thread with channel (for blocking ops) | ||
| let (tx, rx) = std::sync::mpsc::channel(); | ||
| std::thread::spawn(move || { | ||
| let result = blocking_operation(); | ||
| let _ = tx.send(result); | ||
| }); | ||
|
|
||
| cx.spawn(|mut cx| async move { | ||
| if let Ok(data) = rx.recv() { | ||
| cx.update(|cx| { | ||
| // Update UI with data | ||
| }).ok(); | ||
| } | ||
| }).detach(); |
There was a problem hiding this comment.
This async pattern blocks inside the spawned async task (rx.recv()), which can stall the executor. In CloudBridge code, channel waits are wrapped in smol::unblock + recv_timeout (see src/ui/dashboard.rs) to avoid blocking. Adjust this example to use a non-blocking/async receive (or smol::unblock) so it reflects the repo’s working pattern.
Technical Debt Cleanup & Code Optimization
Planned Improvements:
Expected Impact: